Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use quoted collation declaration when available, fixes test suite when running against DBAL 2.10+ #7889

Merged
merged 1 commit into from
Nov 5, 2019

Conversation

ajgarlag
Copy link
Contributor

@ajgarlag ajgarlag commented Nov 5, 2019

Tests are failing with doctrine/dbal >= 2.10 because column collation declaration in MySQL is being quoted (See: doctrine/dbal@733b64b).

$this->assertEquals("CREATE TABLE cms_phonenumbers (phonenumber VARCHAR(50) NOT NULL, user_id INT DEFAULT NULL, INDEX IDX_F21F790FA76ED395 (user_id), PRIMARY KEY(phonenumber)) DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci ENGINE = InnoDB", $sql[7]);
$collation = $this->getColumnCollationDeclarationSQL('utf8_unicode_ci');

$this->assertEquals("CREATE TABLE cms_groups (id INT AUTO_INCREMENT NOT NULL, name VARCHAR(50) NOT NULL, PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8 $collation ENGINE = InnoDB", $sql[0]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's hard to see what changed here - could you revert the diff so that the change is more evident, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @Ocramius, I do not understand your request.

The change is related to collation declaration that now depends on the doctrine/dbal version. You can see related DBAL commit: doctrine/dbal@733b64b

What command should I execute to make the change more evident?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, DBAL version-dependant, sorry!

I'd probably request that you bump doctrine/dbal to ^2.10 in composer.json, and then (if feasible) we keep the strings constant here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, never mind, I see that this is for 2.6 - no version updates there

@ajgarlag ajgarlag force-pushed the hotfix/fix-tests-with-dbal-2.10 branch 3 times, most recently from 8e700fb to 57d44aa Compare November 5, 2019 13:45
@ajgarlag ajgarlag force-pushed the hotfix/fix-tests-with-dbal-2.10 branch from 57d44aa to 1bc4e1f Compare November 5, 2019 13:58
@ajgarlag ajgarlag marked this pull request as ready for review November 5, 2019 14:11
@Ocramius
Copy link
Member

Ocramius commented Nov 5, 2019

Thanks @ajgarlag!

@Ocramius Ocramius added this to the 2.6.5 milestone Nov 5, 2019
@Ocramius Ocramius self-assigned this Nov 5, 2019
@Ocramius Ocramius merged commit adfd010 into doctrine:2.6 Nov 5, 2019
@Ocramius Ocramius changed the title Use quoted collation declaration when available. Use quoted collation declaration when available, fixes test suite when running against DBAL 2.10+ Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants